Use Rule.oneOf to resolve correct loader #2747

Furizaa · 2017-07-08T14:09:33Z

This PR uses the oneOf rule to resolve loaders in webpack.config.prod.js. This will use the first loader that matches in the list of loaders provided by oneOf and enables us to remove the big exclusion list of the file-loader. The file loader will only be used if the module falls through all other loaders.

https://webpack.js.org/configuration/module/#rule-oneof

For some reason though I had to still preserve the js(x) exclusion in the file loader. Otherwise the build would break. This happens with or without the eslint preloader. Will have to investigate further why exactly this happens and why these unknown js(x) modules fall through to the file-loader.

The resolve now looks something like this (pseudocode):

[
  { 'eslint-loader', enforce: 'pre', test: 'javascript' },
  {
    oneOf: [
      { 'url-loader': test: 'images' },
      { 'babel-loader', test: 'javascript' },
      { 'style-loader', test: 'style' },
      { 'file-loader' },
    ]
  }
]

gaearon · 2017-07-08T16:34:21Z

This is sooo sweet. Thank you.
Still pending some work?

Furizaa · 2017-07-08T18:53:05Z

Thank @sokra as he pointed me in the right direction :D Yeah now I see I missed the dev config. Will add that in a bit.

gaearon · 2017-07-08T19:08:48Z

For some reason though I had to still preserve the js(x) exclusion in the file loader. Otherwise the build would break. This happens with or without the eslint preloader. Will have to investigate further why exactly this happens and why these unknown js(x) modules fall through to the file-loader.

Did you figure this part out?

Furizaa · 2017-07-08T19:29:08Z

Did you figure this part out?

Not yet. I can try my luck tomorrow but there is a chance I will come to no conclusion. Debugging webpack is a pain and __webpack_require__(...) is not a function does not really help there.

But -- I added oneOf to the dev config. Should be good now if you don't point me to a spot I missed :)

gaearon · 2017-07-08T21:20:56Z

Let's hang on until you try to figure it out. If you can't, no sweat, just write here and we'll get somebody familiar with webpack internals to look at it. I'd like to understand better before merging. Thank you!

Furizaa · 2017-07-09T09:50:05Z

Ok, I found out what the problem is and have two possible fixes for you. The culprit is the css-loader runtime that gets injected as a .js file that would then be loaded by the file-loader if we don't exclude .js files explicitly ('cause the babel-loader only matches paths.appSrc).

As a proof-of-concept I tried to patch the css-loader so that the runtime gets loaded with the !! prefix which ignores all loaders. But that is another can of worms (weird errors) I was not willing to open.

So here are my proposed solutions:

Keep the exclusion and document why it's there.
(Cleaner IMHO) Add include: paths.appSrc to the file-loader. But this could potentially break projects that use the file-loader to load files from outside paths.appSrc for whatever reason.

I'm happy to implement either one of these based on feedback.

gaearon · 2017-07-09T18:59:38Z

Keep the exclusion and document why it's there.

This sounds good to me.

Furizaa · 2017-07-09T19:53:29Z

Should be good now. Thanks for your precious weekend time guiding me through this. If there is anything else I missed then feel free pointing me to it.

gaearon · 2017-07-09T20:57:50Z

@EnoahNetzach Let me know if you have any comments here!

gaearon · 2017-07-09T20:58:56Z

I’ll get this in for now. Thank you again!

* Use oneOf to resolve correct loader * Add html and json fallthrough again * Use oneOf to resolve correct loader in dev * Document file-loaders `js` exclusion * Remove `jsx` from exclusion in prod config

sokra · 2017-08-09T08:08:12Z

packages/react-scripts/config/webpack.config.dev.js

+            // it's runtime that would otherwise processed through "file" loader.
+            // Also exclude `html` and `json` extensions so they get processed
+            // by webpacks internal loaders.
+            exclude: [/\.js$/, /\.html$/, /\.json$/],


Instead of the exclude you could also add a empty rule matching these types

{ test: [/\.js$/, /\.html$/, /\.json$/] // Don't use a loader for these types. }

This would keep the file-loader rule focused.

Huh. Nice! Thanks for that hint. Will create a PR to change this in time. Seems much cleaner.

* Use oneOf to resolve correct loader * Add html and json fallthrough again * Use oneOf to resolve correct loader in dev * Document file-loaders `js` exclusion * Remove `jsx` from exclusion in prod config

see facebook/create-react-app#2747

* Use oneOf to resolve correct loader * Add html and json fallthrough again * Use oneOf to resolve correct loader in dev * Document file-loaders `js` exclusion * Remove `jsx` from exclusion in prod config

Use oneOf to resolve correct loader

15488ed

facebook-github-bot added the CLA Signed label Jul 8, 2017

Add html and json fallthrough again

34e29c4

gaearon added this to the 1.0.11 milestone Jul 8, 2017

gaearon added the tag: enhancement label Jul 8, 2017

Use oneOf to resolve correct loader in dev

a38d32a

Furizaa added 2 commits July 9, 2017 21:15

Document file-loaders js exclusion

3345b80

Remove jsx from exclusion in prod config

4963482

gaearon merged commit a08eb3b into facebook:master Jul 9, 2017

sokra reviewed Aug 9, 2017

View reviewed changes

tb added a commit to tb/react-app-rewire-graphql-tag that referenced this pull request Oct 15, 2017

Flatten rules after oneOf was added to create-react-app webpack configs

1ed239f

see facebook/create-react-app#2747

tb added a commit to tb/react-app-rewire-graphql-tag that referenced this pull request Oct 15, 2017

Flatten rules after oneOf was added to create-react-app webpack configs

8c89cf5

see facebook/create-react-app#2747

tb mentioned this pull request Oct 15, 2017

Flatten rules after oneOf was added to create-react-app webpack configs tkvw/react-app-rewire-graphql-tag#1

Merged

davidtheclark mentioned this pull request Mar 24, 2018

Use file-loader as fallback for files that match no other loader mapbox/batfish#257

Closed

lock bot locked and limited conversation to collaborators Jan 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Rule.oneOf to resolve correct loader #2747

Use Rule.oneOf to resolve correct loader #2747

Furizaa commented Jul 8, 2017

gaearon commented Jul 8, 2017

Furizaa commented Jul 8, 2017

gaearon commented Jul 8, 2017

Furizaa commented Jul 8, 2017

gaearon commented Jul 8, 2017

Furizaa commented Jul 9, 2017 •

edited

Loading

gaearon commented Jul 9, 2017

Furizaa commented Jul 9, 2017

gaearon commented Jul 9, 2017

gaearon commented Jul 9, 2017

sokra Aug 9, 2017

Furizaa Aug 9, 2017

Use Rule.oneOf to resolve correct loader #2747

Use Rule.oneOf to resolve correct loader #2747

Conversation

Furizaa commented Jul 8, 2017

gaearon commented Jul 8, 2017

Furizaa commented Jul 8, 2017

gaearon commented Jul 8, 2017

Furizaa commented Jul 8, 2017

gaearon commented Jul 8, 2017

Furizaa commented Jul 9, 2017 • edited Loading

gaearon commented Jul 9, 2017

Furizaa commented Jul 9, 2017

gaearon commented Jul 9, 2017

gaearon commented Jul 9, 2017

sokra Aug 9, 2017

Choose a reason for hiding this comment

Furizaa Aug 9, 2017

Choose a reason for hiding this comment

Furizaa commented Jul 9, 2017 •

edited

Loading